gh-156439: Gate socket.if_{nametoindex, indextoname} on respective OS capability - #156708
Conversation
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
All commit authors signed the Contributor License Agreement. |
Co-authored-by: Stan Ulbrych <stan@python.org>
StanFromIreland
changed the title
socket.if_{nametoindex, indextoname} on respective OS capability
StanFromIreland
changed the title
socket.if_{nametoindex, indextoname} on respective OS capabilitysocket.if_{nametoindex, indextoname} on respective OS capability
Co-authored-by: Stan Ulbrych <stan@python.org>
| # define NET_IFINDEX unsigned int | ||
| # endif | ||
| #endif // defined(HAVE_IF_NAMEINDEX) || defined(MS_WINDOWS) | ||
| #endif // defined(HAVE_IF_INDEXTONAME) || defined(MS_WINDOWS) |
There was a problem hiding this comment.
This comment is mismatched, but I also don't see why we need HAVE_IF_NAMETOINDEX? I understand this was @mhsmith 's suggestion, can you please clarify why? From my understanding, it's not necessary, the NET_IFINDEX/_PyLong_NetIfindex_Converter definitions are only needed by if_indextoname.
There was a problem hiding this comment.
It's used in the function body:
cpython/Modules/socketmodule.c
Lines 7418 to 7423 in e0afadb
There was a problem hiding this comment.
Unless that's something that's defined in the Windows headers?
There was a problem hiding this comment.
Unless that's something that's defined in the Windows headers?
From what I can tell, it's defined by Iphlpapi.h on Windows (see docs for if_indextoname). As such, it's conditional on #ifdef MS_WINDOWS. Adding HAVE_IF_NAMETOINDEX makes no difference here.
|
Merged, thanks all! |
3 participants
I'm not exactly a C programmer so hopefully I'm approaching this the correct way?
if_indextonameandif_nametoindexwithoutif_nameindex, but socket doesn't expose that #156439